Skip to content

test(agc): check eager non-NGG registration with a program no host can prepare - #4142

Closed
Tamalero wants to merge 1 commit into
boykopovar:mainfrom
Tamalero:fix/registered-workgroup-sync-control
Closed

test(agc): check eager non-NGG registration with a program no host can prepare#4142
Tamalero wants to merge 1 commit into
boykopovar:mainfrom
Tamalero:fix/registered-workgroup-sync-control

Conversation

@Tamalero

@Tamalero Tamalero commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

What

agc_driver_prepared_shaders_registered_workgroup_sync fails on main (bca7a29d) on 32-lane hosts with expected preparation failure. It is also reported in the bodies of #4085 (RTX 4060, lavapipe) and #4120.

Cause: two merges landed ten minutes apart. #2261's control registers a non-NGG copy of the registered-workgroup fixture in synchronous mode. It expected the lane-63 refusal (v_readlane_b32 of lane 63 is outside) as proof that registration prepared the program eagerly. #3546 then made a wave64 vertex program's lane-63 read build on a 32-lane host, so the refusal no longer comes and nothing else throws.

Fix (test only): the control registers a program that a vertex stage cannot build on any host width: the fixture's own ds_write_b32 followed by s_barrier, which EmitBarrier refuses (s_barrier in a vertex program that accesses LDS or GDS or writes memory is not implemented, SpirvFlowEmitter.cpp). The rest of the test is unchanged. The subgroup gate (subgroupSize < 64) is no longer needed, so the control now also runs on 64-lane hosts.

Tested

CachyOS (Linux 7.2), GCC, Release, RTX 4090 (NVIDIA 615.78, 32-lane subgroups), on main bca7a29d.

  • agc_driver_prepared_shaders*: 9/9 pass. On main without this change, 8/9: _registered_workgroup_sync fails as above.
  • Negative checks, with only ShaderRegistry.cpp mutated and restored byte for byte afterwards:
    • NGG deferral removed (if (false) at the registration-deferral line): _registered_workgroup and _sync both fail (missing AGC user-data header).
    • Every vertex registration deferred (routing and workgroup conditions replaced by true): both fail (expected preparation failure).
  • tools/check_conventions.py --base origin/main and git diff --check origin/main pass.

Not tested: Windows, lavapipe, a 64-lane host. The control has no subgroup or mesh-shading gate, and the program it registers is refused before any mesh path is involved.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

Tested and written by Claude (AI assistant) on a contributor's machine.

@Tamalero @claude
test(agc): check eager non-NGG registration with a program no host ca…
425a9af 
…n prepare

The registered-workgroup control registers a non-NGG copy of its fixture
in synchronous mode and expected the lane-63 refusal to prove that
preparation ran at registration. Since boykopovar#3546 a wave64 vertex program's
lane-63 read builds on a 32-lane host, so the refusal never comes and
agc_driver_prepared_shaders_registered_workgroup_sync fails on main on
every host narrower than 64 lanes.

The control now registers an LDS write followed by s_barrier, which a
vertex program cannot build on any host width, so it also runs on
64-lane hosts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Tamalero Tamalero mentioned this pull request Oct 11, 2026
Merged
9 tasks done
@Dextroid17

Dextroid17 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

The replacement control does not depend on host width, unlike the lane-63 v_readlane_b32 refusal it replaces, and the message it keys on is already relied on elsewhere (SpirvFlowEmitter.cpp "s_barrier in a vertex program that accesses LDS or GDS or writes memory is not implemented", matched by tests/VertexBarrier.cpp:46), so the expectation is not a new coupling.

What I checked in the tree:

  • the fixture's BDA registers are {0xc8, address >> 8} / {0xc9, address >> 40}; the control writes both from the same address as shader.code/shader_size, so the registers and the code field name the same 4-word program the driver walks.
  • 0xd8340000 + a second word, 0xbf8a0000 (s_barrier), 0xbf810000 (s_endpgm) is a ds_write_b32 followed by a barrier, which is the "writes memory" case, not merely "has a barrier".
  • alignas(256) on the new array is load-bearing, not stylistic — the driver rejects a misaligned code pointer, so dropping it would change what's being tested.

The subgroupSize < 64 gate had been skipping the expectation entirely; the diff removes it, so the control also runs on 64-lane hosts, a case the body lists as not run.

One dependency worth naming for anyone touching this test later: the control relies on header.specials.vgt_shader_stages_en.value = 0 from the line just above it. With NGG routing (the field's original value) the same program would be deferred at registration instead of refused immediately, and the expectation would need to change to match.

Written with AI assistance.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 12, 2026
@oneandonlydean

oneandonlydean commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Closing: #4177 fixed agc_driver_prepared_shaders_registered_workgroup_sync on main; the fixture now reads lane 63 of the vertex index, which still throws after #3546. Please reopen if it fails for you on current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agc libSceAgc*

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Tamalero @Dextroid17 @oneandonlydean